Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/design/agent-skills/agent-skills-spike.md`:
- Around line 297-304: The SkillConfiguration model currently only enforces
length for the name field; add a Pydantic field validator on SkillConfiguration
for the name attribute (using field_validator('name') as a `@classmethod`, e.g.,
validate_name_format) that checks the regex ^[a-z0-9-]+$ and raises a ValueError
if it fails, preserving the existing min_length/max_length constraints so names
remain 1-64 chars but now also must be lowercase letters, numbers, and hyphens
only.
- Line 9: Update the "PoC validation" section header and sentence: replace the
confusing line starting with "**PoC validation**: Not applicable for this
spike." followed by the product list with a clear validation statement such as
"PoC validation: Validated by 30+ production implementations including Claude
Code, GitHub Copilot, Cursor, and OpenAI Codex" so the intent (that the standard
is proven) is explicit; edit the sentence under the "PoC validation" header to
use this rewording.
- Around line 133-135: The comment flags fragile line-number references; update
the docs to remove hard-coded line numbers and instead reference durable symbols
like the Configuration class definition in src/models/config.py and the
ModelContextProtocolServer class (MCP server config pattern), or add a note that
any line numbers should be verified/updated if code moves; ensure all
occurrences in agent-skills-spike.md use class names or file+symbol references
rather than specific line numbers.
- Around line 145-148: Clarify in the design/acceptance notes whether
build_skill_catalog() output is appended to per-request system prompts returned
by get_system_prompt() or only to resolved default prompts: update the
implementation plan and acceptance criteria to state the exact behavior (e.g.,
"skills are injected into the final resolved system prompt even when a
per-request system_prompt is provided" or "providing a per-request system_prompt
disables skill injection"), and mention interaction with the
disable_query_system_prompt flag (which when True prevents per-request prompts
and skill injection). Reference get_system_prompt(), build_skill_catalog(), and
disable_query_system_prompt in the note so reviewers and implementers know
exactly where to enforce or check this behavior.

In `@docs/design/agent-skills/agent-skills.md`:
- Around line 164-165: Add a brief comment above the PrivateAttr declarations
for _content and _references explaining these fields are runtime-only caches
(not serialized), are populated once during startup config validation (from
parsing SKILL.md and scanning the references directory), and should not be
relied on for persisted state; reference the _content, _references fields and
the use of PrivateAttr so future maintainers know their purpose and lifecycle.
- Around line 264-276: The pseudo-code in the PR appends the skill catalog to a
non-existent `resolved_prompt` variable but the real get_system_prompt() has
multiple early returns; update get_system_prompt() to first compute and store
the resolved prompt in a single variable (e.g., `base_prompt`) following the
existing precedence (explicit system_prompt, customization.custom_profile
default, customization.system_prompt, constants.DEFAULT_SYSTEM_PROMPT), then
call build_skill_catalog(configuration.skills) and if it returns content append
it to `base_prompt` and return that combined string, otherwise return
`base_prompt`; reference the get_system_prompt function, the build_skill_catalog
call, configuration.skills, and constants.DEFAULT_SYSTEM_PROMPT when making the
change.
- Around line 412-416: The current is_skill_content function uses simple
substring checks which can produce false positives; update is_skill_content to
use a regex that matches the full tag structure instead: compile and use the
pattern r'<skill_content\s+name="[^"]+">.*</skill_content>' with the DOTALL flag
(and import re if not present) and return whether the regex finds a match; keep
the function name is_skill_content and ensure the regex-based check replaces the
existing substring checks.
- Around line 423-439: SkillTracker currently keeps an unbounded _activated dict
(class SkillTracker, methods is_activated, mark_activated, clear) which can leak
memory; update it so entries are removed when conversations end and stale
entries expire: hook clear(conversation_id) into the conversation
deletion/cleanup pipeline, and replace or wrap _activated with a size-limited
LRU or TTL cache (e.g., implement an LRU eviction policy with a max_entries
parameter and/or per-entry expiry timestamps to purge abandoned conversations)
so that mark_activated and is_activated operate against the capped/expiring
storage and expired entries are evicted on access or by a periodic cleanup.
- Around line 520-531: The list_references function currently returns files
under refs_dir without excluding symbolic links, allowing symlinks to point
outside the skill directory; update list_references (the function and its use of
refs_dir and the rglob/f.is_file() check) to skip symlinks by filtering out
entries where f.is_symlink() is True (or use
f.resolve().is_relative_to(skill_path) as an extra safety check) so only regular
files inside the skill_path/references tree are returned; ensure the returned
paths remain relative_to(skill_path).
- Line 485: The line currently references the wrong tool name `read_skill`;
update it to the correct tool name `activate_skill` so the document consistently
uses `activate_skill` (verify and replace any stray `read_skill` mentions with
`activate_skill` in the same section), ensuring references to the tool handler
and function type match the rest of the doc and examples.
- Around line 167-186: In validate_skill_path (the `@model_validator` method
validate_skill_path) add an explicit check that Path(self.path) exists and is a
directory before checking for SKILL.md: compute skill_dir = Path(self.path), if
not skill_dir.exists() or not skill_dir.is_dir() raise a ValueError like "Skill
path not found or not a directory: {skill_dir}" so callers get a clear error;
then continue to build skill_md_path = skill_dir / "SKILL.md" and keep the
existing parse_skill_md, name comparison, setting of self._content and
self._references (list_references(Path(self.path))).
- Around line 327-354: The tool response in handle_activate_skill interpolates
unescaped values into XML; escape XML special characters for skill.name
(attribute), skill._content (body), skill.path, and each item in
skill._references before building lines. Use a proper XML/HTML escape utility
(e.g., html.escape or xml.sax.saxutils.escape) to replace &, <, >, ", ' and
apply it where skill.name, skill._content, skill.path and ref are inserted so
that the attribute, element text, and file entries are safe; keep the same tags
and structure but feed escaped strings into the list assembly.
- Around line 494-517: Update parse_skill_md to use a more flexible frontmatter
regex and stronger field validation: relax the regex to accept CRLF or LF line
endings and optional whitespace after the closing --- (e.g., use \r?\n and allow
whitespace or no trailing newline so frontmatter/body split still works), keep
re.DOTALL, and fall back to a match that allows end-of-file without an extra
newline; then validate that frontmatter (the YAML-safe_load result) is a dict
and that frontmatter["name"] and frontmatter["description"] exist, are instances
of str, and are non-empty after stripping (raise clear ValueError messages if
not). Ensure you update error messages from parse_skill_md to mention the
specific missing/invalid field names and include the variable names frontmatter
and body in the checks to make locating the code easier.
- Around line 252-258: The loop that builds XML for each skill currently
interpolates skill.name and skill.description directly (see the for skill in
skills: block) which can produce malformed XML or injection; update this to
escape XML special characters before interpolation (e.g., call a sanitizer like
html.escape or xml.sax.saxutils.escape on skill.name and skill.description) so
the lines appended use the escaped values; ensure both name and description are
escaped and keep the surrounding tags intact in the block that extends lines.